Skip to content

Symbol visibility controls and exports (remove-internal approach) - #1625

Draft
ramakrishnap-nv wants to merge 27 commits into
mainfrom
fix-symbol-visibility-remove-internal
Draft

Symbol visibility controls and exports (remove-internal approach)#1625
ramakrishnap-nv wants to merge 27 commits into
mainfrom
fix-symbol-visibility-remove-internal

Conversation

@ramakrishnap-nv

Copy link
Copy Markdown
Collaborator

Summary

Enables symbol visibility controls (hidden-by-default) and explicit exports across the cuOpt C/C++/Cython API surface, and removes the internal-symbol test-linkage workaround by consolidating internal tests.

Based on @arhag23's fix-symbol-visibility-remove-internal, with main merged in. This is the remove-internal approach — an alternative to #1526 (fix-symbol-visibility-2, which keeps cuopt_static and minimizes its linkage).

Key changes

  • Hidden-by-default symbol visibility with explicit CUOPT_EXPORT annotations across public headers (mathematical_optimization, routing, io, grpc, error).
  • Version-script / export controls in cpp/CMakeLists.txt and cpp/include/cuopt/export.hpp.
  • check symbols script for CI verification.
  • Test refactors: consolidate internal numopt tests into a single binary, fix combined-binary linkage, remove stale cuopt_static internal-test linkage stubs.
  • Assorted build fixes (OpenMP/simde PRIVATE deps, PSLP linkage, RAPIDS pin).

Status

  • ✅ Up to date with main (0 commits behind at time of push).
  • Merge conflicts resolved: cuopt_c.h (main's new generic-attribute getters kept inside the visibility push/pop) and c_api_tests.cpp (kept main's gmock include + mixed-precision decl).
  • Opened as draft pending review of scope vs. Fix symbol visibility and add checks #1526.

Supersedes draft #1620, which pointed at the fork branch that could not be updated directly.

ramakrishnap-nv and others added 20 commits July 20, 2026 12:38
Prerequisite structural work for enabling -fvisibility=hidden on
libcuopt.so (issue #1213).

- Introduce cuopt_objs OBJECT library so cuopt (shared) and
  cuopt_static (static, BUILD_TESTS only) share one compile step
- Add STATIC_LIB option to ConfigureTest; when set, links cuopt_static
  and omits cuopttestutils to avoid transitive cuopt-shared pull-in
- Route all tests with compiled internal symbol dependencies through
  STATIC_LIB: DUAL_SIMPLEX_TEST, LP_INTERNAL_TEST, PDLP_TEST,
  MPS_PARSER_TEST, most MIP tests, SOCP_TEST, ROUTING_INTERNAL_TEST
- Create ROUTING_INTERNAL_TEST consolidating GES, scross, local-search
  candidate, and top-k tests; register previously unbuilt test files
  (l0_scross_test.cu, local_search_cand_test.cu, load_balancing_test.cu,
  feasibility_jump_tests.cu)
- Add gRPC source files directly to GRPC_CLIENT_TEST and
  GRPC_INTEGRATION_TEST so they do not depend on internal symbols being
  exported from the shared library
- Promote is_cusparse_runtime_mixed_precision_supported() to the public
  C API as cuOptIsCusparseRuntimeMixedPrecisionSupported(); update
  c_api_tests.cpp to use the public function

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
Collapse 19 separate STATIC_LIB test binaries (dual_simplex, LP internal,
PDLP, MPS parser, 11 MIP, SOCP) into a single NUMOPT_INTERNAL_TEST binary
linked against cuopt_static.  Routing internal tests remain in
ROUTING_INTERNAL_TEST.  This reduces total static-linked binary count from
19 to 2, cutting link time and disk usage.

Also fix cuopt_c.cpp to forward-declare
is_cusparse_runtime_mixed_precision_supported() instead of including
cusparse_view.hpp, which pulled CUDA intrinsics into a CXX translation unit.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
Fix four issues discovered when building NUMOPT_INTERNAL_TEST and
ROUTING_INTERNAL_TEST:

- omp_helpers.hpp: closing namespace brace was outside #ifdef _OPENMP,
  causing "expected a declaration" when compiled without -fopenmp.

- cuopt_static: propagate OpenMP::OpenMP_CXX PUBLIC and -fopenmp for
  CUDA TUs so test binaries that link cuopt_static compile OMP-heavy
  internal headers correctly.

- cuopt shared: restore CUSPARSE_ENABLE_EXPERIMENTAL_API and
  CUOPT_LOG_ACTIVE_LEVEL as PUBLIC compile definitions; they were lost
  when compile definitions moved from cuopt to cuopt_objs (the
  $<TARGET_OBJECTS:...> pattern does not carry INTERFACE properties).

- Combined binaries: add CUOPT_DISABLE_TEST_MAIN guard to
  CUOPT_TEST_PROGRAM_MAIN() in base_fixture.hpp; provide a single
  main.cu entry point per binary; make init_handler static in three
  MIP test files to resolve ODR violations; compile check_constraints.cu
  directly into ROUTING_INTERNAL_TEST for check_route; add missing
  src/ and src/io/ include paths to GRPC_CLIENT_TEST and
  GRPC_INTEGRATION_TEST.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
All tests from these directories moved to NUMOPT_INTERNAL_TEST.
The subdirectories and add_subdirectory calls serve no purpose now.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
$<TARGET_OBJECTS:cuopt_objs> does not propagate PRIVATE link
dependencies to consuming targets; PSLP must be linked explicitly
on both cuopt_static and cuopt (shared).

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
…test files

Files compiled into NUMOPT_INTERNAL_TEST and ROUTING_INTERNAL_TEST no
longer need their own main(); remove the call directly rather than
suppressing it via the CUOPT_DISABLE_TEST_MAIN guard. Simplifies
base_fixture.hpp and the CMakeLists files.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
Remove cuOptIsCusparseRuntimeMixedPrecisionSupported from the public
C API. The c_api_test already includes internal headers; use the
compile-time CUSPARSE_VERSION macro instead, which is accurate for
cuOpt's deployment model (compile-time and runtime toolkit match).

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
When cuSPARSE < 12.5, cuopt_expects throws ValidationError before
the solve begins, so the C API always returns an error — CUOPT_SUCCESS
with non-optimal is impossible. Assert status != CUOPT_SUCCESS directly.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
After thrust::exclusive_scan, offsets.back() already equals the total
number of non-zeros (standard CSR sentinel). The previous + 1 produced
one entry beyond the last row, which is invalid CSR and caused
cusparseXcsrsort to produce incorrect results when assertions are
enabled (PR CI uses -a / DEFINE_ASSERT=ON which enables -UNDEBUG).

The resulting out-of-bounds column index triggered the
check_csr_representation assertion in compute_transpose_of_problem,
crashing NUMOPT_INTERNAL_TEST with SIGABRT in PR CI.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
The compile-time #if CUSPARSE_VERSION >= 12500 guard only checks the
header version used during build. On systems where the runtime cuSPARSE
library is older than 12.5 (e.g., oldest-deps conda builds), the solver
correctly throws ValidationError via is_cusparse_runtime_mixed_precision_supported(),
but the test unconditionally expected CUOPT_SUCCESS.

Fix by calling cusparseGetProperty() at runtime in the test to mirror
the same check the solver performs, routing the test to the success or
failure assertion accordingly.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
top_k.cu was moved to ROUTING_INTERNAL_TEST because it uses internal
symbols (routing/detail). It was also the only source in ROUTING_UNIT_TEST
that provided CUOPT_TEST_PROGRAM_MAIN(), which sets up the RMM device
memory resource before running tests.

Without RMM setup, the routing solver allocations use the default
cuda_memory_resource, causing SIGSEGV in vehicle_breaks.non_uniform_breaks
(and potentially other tests that stress the allocator).

Fix by adding internal/main.cu to ROUTING_UNIT_TEST sources. The
custom main() it provides takes precedence over GTest::gmock_main's
default main and properly initializes the RMM memory resource.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
The SPDX header used bare `/* text` lines without closing `*/` on each
line, leaving an unclosed C comment. Subsequent `/*` markers triggered
-Werror=comment when internal/main.cu was compiled into new targets
(e.g. ROUTING_UNIT_TEST). Use the standard block-comment style matching
the rest of the test suite.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
cuopt_objs is the OBJECT library that compiles all CUDA TUs. Before
the cuopt_objs refactor, cuopt SHARED had OpenMP::OpenMP_CUDA as a
PRIVATE dep, which passed -fopenmp to nvcc and defined _OPENMP for
all .cu files. After the refactor, cuopt_objs only inherited
OpenMP::OpenMP_CXX (via CUOPT_PRIVATE_CUDA_LIBS), which only applies
to CXX compilation. Without -fopenmp, omp_atomic_t<T> is undefined
in CUDA TUs that include omp_helpers.hpp (e.g. via
dual_simplex/solution.hpp), causing build failures.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
The fast MPS parser sources include simde/x86/avx2.h. Before the
cuopt_objs refactor, simde::simde was a PRIVATE dep of cuopt SHARED
and its include path was available during source compilation. After
the refactor, sources are compiled in cuopt_objs, so simde::simde
must be listed there.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
…test

QcMatrixRowsMatchReferenceBitwise provided X1 X2 -2.5 but not the
required matching X2 X1 -2.5. The classic MPS parser enforces that
each off-diagonal (i,j) entry has an explicit (j,i) counterpart;
verify_fixture_bitwise routes through both parsers and the reference
parser was throwing ValidationError on the asymmetric input.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
…ty-remove-internal

# Conflicts:
#	cpp/include/cuopt/mathematical_optimization/cuopt_c.h
#	cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp
@copy-pr-bot

copy-pr-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@ramakrishnap-nv ramakrishnap-nv self-assigned this Jul 24, 2026
@ramakrishnap-nv ramakrishnap-nv added non-breaking Introduces a non-breaking change improvement Improves an existing functionality labels Jul 24, 2026
@ramakrishnap-nv

Copy link
Copy Markdown
Collaborator Author

/ok to test b1644c1

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

CI Test Summary

1 failed · 30 passed · 0 skipped

ramakrishnap-nv and others added 3 commits July 27, 2026 11:06
With hidden-by-default visibility, tests that reference internal symbols
fail to link against the shared cuopt library (undefined references to the
routing dataset generator, mps fast-parser internals, and grpc mapper
helpers such as canonicalize_coo_matrix). This broke conda-cpp-build, which
builds the gtest binaries; wheel builds were unaffected because they don't.

Route these tests through the existing STATIC_LIB mechanism so they link
cuopt_static:
- routing: ROUTING_TEST, VEHICLE_ORDER_TEST, VEHICLE_TYPES_TEST,
  OBJECTIVE_FUNCTION_TEST, RETAIL_L1TEST, ROUTING_L1TEST, ROUTING_UNIT_TEST
- linear_programming: MPS_FAST_PARSER_TEST
- grpc: GRPC_CLIENT_TEST, GRPC_PIPE_SERIALIZATION_TEST, GRPC_INTEGRATION_TEST

Add cuopttestutils_static (check_constraints.cu linked against cuopt_static)
so STATIC_LIB tests resolve test-utility symbols like check_route without
mixing shared and static cuopt in one binary, and wire it into the
ConfigureTest STATIC_LIB path. ROUTING_INTERNAL_TEST no longer needs to
compile check_constraints.cu directly.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
…ty-remove-internal

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>

# Conflicts:
#	cpp/CMakeLists.txt
#	cpp/tests/CMakeLists.txt
#	cpp/tests/routing/CMakeLists.txt
@ramakrishnap-nv

Copy link
Copy Markdown
Collaborator Author

/ok to test 0db00c3

Addresses review feedback on #1581: the trailing ' *' before the closing
'*/' in the CUOPT_TEST_PROGRAM_MAIN doc block serves no purpose.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
@ramakrishnap-nv

Copy link
Copy Markdown
Collaborator Author

/ok to test 969511b

The conda libcuopt recipe invokes ./ci/check_symbols.sh directly (recipe.yaml),
which requires the executable bit. It was committed as 100644, so conda-cpp-build
failed with 'Permission denied' once the earlier link errors were resolved.
All sibling ci/*.sh scripts are 100755.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
@ramakrishnap-nv

Copy link
Copy Markdown
Collaborator Author

/ok to test b0ce83e

@ramakrishnap-nv

Copy link
Copy Markdown
Collaborator Author

/ok to test d51eaed

Each STATIC_LIB test binary embeds the full multi-arch cuopt_static device
code, so giving every standalone routing test its own static binary exploded
the libcuopt-tests conda package to ~4.24 GiB. Fold the active level-0 routing
tests (ROUTING_TEST, VEHICLE_ORDER_TEST, VEHICLE_TYPES_TEST,
OBJECTIVE_FUNCTION_TEST) into the existing consolidated ROUTING_INTERNAL_TEST
static binary instead of 4 separate ones, matching the pattern documented in
cpp/tests/internal/CMakeLists.txt. Drop CUOPT_TEST_PROGRAM_MAIN() from the
folded files since the combined binary provides main via internal/main.cu
(same treatment l0_ges_test.cu already has).

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
@ramakrishnap-nv

Copy link
Copy Markdown
Collaborator Author

/ok to test 2980061

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants